From 50e00c9712edb40103a69f262b8f065f79a893d4 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 19 Jan 2015 15:46:07 -0800 Subject: [PATCH] Augment dylib search paths for rustc/build scripts Whenever a build script or a plugin depends on a native dynamic library, the dylib search path will be required to include the path to the generated dynamic library. Currently Cargo does not augment the search path, causing load errors or execution errors whenever builds are attempted in this case. This commit remedies the situation by ensuring that dynamic libraries show up in dylib search paths. This commit is necessary to fully migrate the remaining tests from the old build command infrastructure to build scripts. --- Cargo.toml | 5 - src/cargo/ops/cargo_rustc/custom_build.rs | 17 ++- src/cargo/ops/cargo_rustc/engine.rs | 7 ++ src/cargo/ops/cargo_rustc/mod.rs | 134 ++++++++++++++-------- tests/test_cargo_compile_custom_build.rs | 74 +++++++++++- tests/test_cargo_compile_plugins.rs | 27 ++--- 6 files changed, 191 insertions(+), 73 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 8ff3ffba1..d6215a66b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -5,11 +5,6 @@ authors = ["Yehuda Katz ", "Carl Lerche ", "Alex Crichton "] -[profile.test] -debug = false -[profile.dev] -debug = false - [lib] name = "cargo" path = "src/cargo/lib.rs" diff --git a/src/cargo/ops/cargo_rustc/custom_build.rs b/src/cargo/ops/cargo_rustc/custom_build.rs index 54b633d58..e5af16fb2 100644 --- a/src/cargo/ops/cargo_rustc/custom_build.rs +++ b/src/cargo/ops/cargo_rustc/custom_build.rs @@ -4,7 +4,6 @@ use std::io::fs::PathExtensions; use std::io::{fs, USER_RWX, File}; use std::str; use std::sync::Mutex; -use std::sync::mpsc::Sender; use core::{Package, Target, PackageId, PackageSet}; use util::{CargoResult, human, Human}; @@ -26,8 +25,10 @@ pub struct BuildOutput { pub metadata: Vec<(String, String)>, } +pub type BuildMap = HashMap<(PackageId, Kind), BuildOutput>; + pub struct BuildState { - pub outputs: Mutex>, + pub outputs: Mutex, } /// Prepares a `Work` that executes the target as a custom build script. @@ -100,6 +101,7 @@ pub fn prepare(pkg: &Package, target: &Target, req: Platform, let id = pkg.get_package_id().clone(); let all = (id.clone(), pkg_name.clone(), build_state.clone(), build_output.clone()); + let plugin_deps = super::crawl_build_deps(cx, pkg, target, Kind::Host); try!(fs::mkdir_recursive(&cx.layout(pkg, Kind::Target).build(pkg), USER_RWX)); try!(fs::mkdir_recursive(&cx.layout(pkg, Kind::Host).build(pkg), USER_RWX)); @@ -111,7 +113,7 @@ pub fn prepare(pkg: &Package, target: &Target, req: Platform, // // Note that this has to do some extra work just before running the command // to determine extra environment variables and such. - let work = move |: desc_tx: Sender| -> CargoResult<()> { + let work = Work::new(move |desc_tx| { // Make sure that OUT_DIR exists. // // If we have an old build directory, then just move it into place, @@ -124,7 +126,9 @@ pub fn prepare(pkg: &Package, target: &Target, req: Platform, } // For all our native lib dependencies, pick up their metadata to pass - // along to this custom build command. + // along to this custom build command. We're also careful to augment our + // dynamic library search path in case the build script depended on any + // native dynamic libraries. let mut p = p; { let build_state = build_state.outputs.lock().unwrap(); @@ -137,6 +141,7 @@ pub fn prepare(pkg: &Package, target: &Target, req: Platform, Some(value.as_slice())); } } + p = try!(super::add_plugin_deps(p, &*build_state, plugin_deps)); } // And now finally, run the build command itself! @@ -167,7 +172,7 @@ pub fn prepare(pkg: &Package, target: &Target, req: Platform, })); Ok(()) - }; + }); // Now that we've prepared our work-to-do, we need to prepare the fresh work // itself to run when we actually end up just discarding what we calculated @@ -181,7 +186,7 @@ pub fn prepare(pkg: &Package, target: &Target, req: Platform, let (freshness, dirty, fresh) = try!(fingerprint::prepare_build_cmd(cx, pkg, kind, Some(target))); let dirty = Work::new(move |tx| { - try!(work(tx.clone())); + try!(work.call((tx.clone()))); dirty.call(tx) }); let fresh = Work::new(move |tx| { diff --git a/src/cargo/ops/cargo_rustc/engine.rs b/src/cargo/ops/cargo_rustc/engine.rs index e81058139..366753c82 100644 --- a/src/cargo/ops/cargo_rustc/engine.rs +++ b/src/cargo/ops/cargo_rustc/engine.rs @@ -2,6 +2,7 @@ use std::collections::HashMap; use std::ffi::CString; use std::fmt::{self, Formatter}; use std::io::process::ProcessOutput; +use std::os; use std::path::BytesContainer; use util::{self, CargoResult, ProcessError, ProcessBuilder}; @@ -84,6 +85,12 @@ impl CommandPrototype { self } + pub fn get_env(&self, var: &str) -> Option { + self.env.get(var).cloned().or_else(|| { + Some(os::getenv(var).map(|s| CString::from_vec(s.into_bytes()))) + }).and_then(|val| val) + } + pub fn get_envs(&self) -> &HashMap> { &self.env } diff --git a/src/cargo/ops/cargo_rustc/mod.rs b/src/cargo/ops/cargo_rustc/mod.rs index a5d563398..d50bc0ede 100644 --- a/src/cargo/ops/cargo_rustc/mod.rs +++ b/src/cargo/ops/cargo_rustc/mod.rs @@ -1,6 +1,8 @@ use std::collections::{HashSet, HashMap}; use std::dynamic_lib::DynamicLibrary; +use std::ffi::CString; use std::io::fs::{self, PathExtensions}; +use std::os; use std::path; use std::sync::Arc; @@ -16,7 +18,7 @@ pub use self::context::Context; pub use self::context::Platform; pub use self::engine::{CommandPrototype, CommandType, ExecEngine, ProcessEngine}; pub use self::layout::{Layout, LayoutProxy}; -pub use self::custom_build::BuildOutput; +pub use self::custom_build::{BuildOutput, BuildMap}; mod context; mod compilation; @@ -333,7 +335,9 @@ fn rustc(package: &Package, target: &Target, let crate_types = target.rustc_crate_types(); let rustcs = try!(prepare_rustc(package, target, crate_types, cx, req)); - rustcs.into_iter().map(|(rustc, kind)| { + let plugin_deps = crawl_build_deps(cx, package, target, Kind::Host); + + return rustcs.into_iter().map(|(rustc, kind)| { let name = package.get_name().to_string(); let is_path_source = package.get_package_id().get_source_id().is_path(); let show_warnings = package.get_package_id() == cx.resolve.root() || @@ -346,37 +350,12 @@ fn rustc(package: &Package, target: &Target, // Prepare the native lib state (extra -L and -l flags) let build_state = cx.build_state.clone(); - let mut native_lib_deps = HashSet::new(); let current_id = package.get_package_id().clone(); - + let plugin_deps = plugin_deps.clone(); + let mut native_lib_deps = crawl_build_deps(cx, package, target, kind); if package.has_custom_build() && !target.get_profile().is_custom_build() { - native_lib_deps.insert(current_id.clone()); + native_lib_deps.insert(0, current_id.clone()); } - // Visit dependencies transitively to figure out what our native - // dependencies are (for -L and -l flags). - // - // Be sure to only look at targets of the same Kind, however, as we - // don't want to include native libs of plugins for targets for example. - fn visit<'a>(cx: &'a Context, pkg: &'a Package, target: &Target, - kind: Kind, - visiting: &mut HashSet<&'a PackageId>, - libs: &mut HashSet) { - for &(pkg, target) in cx.dep_targets(pkg, target).iter() { - let req = cx.get_requirement(pkg, target); - if !req.includes(kind) { continue } - if !visiting.insert(pkg.get_package_id()) { continue } - - if pkg.has_custom_build() { - libs.insert(pkg.get_package_id().clone()); - } - visit(cx, pkg, target, kind, visiting, libs); - visiting.remove(&pkg.get_package_id()); - } - } - visit(cx, package, target, kind, - &mut HashSet::new(), &mut native_lib_deps); - let mut native_lib_deps = native_lib_deps.into_iter().collect::>(); - native_lib_deps.sort(); // If we are a binary and the package also contains a library, then we // don't pass the `-l` flags. @@ -392,21 +371,15 @@ fn rustc(package: &Package, target: &Target, let mut rustc = rustc; // Only at runtime have we discovered what the extra -L and -l - // arguments are for native libraries, so we process those here. - { - let build_state = build_state.outputs.lock().unwrap(); - for id in native_lib_deps.into_iter() { - let output = &build_state[(id.clone(), kind)]; - for path in output.library_paths.iter() { - rustc = rustc.arg("-L").arg(path); - } - if pass_l_flag && id == current_id { - for name in output.library_links.iter() { - rustc = rustc.arg("-l").arg(name.as_slice()); - } - } - } - } + // arguments are for native libraries, so we process those here. We + // also need to be sure to add any -L paths for our plugins to the + // dynamic library load path as a plugin's dynamic library may be + // located somewhere in there. + let build_state = build_state.outputs.lock().unwrap(); + rustc = add_native_deps(rustc, &*build_state, native_lib_deps, + kind, pass_l_flag, ¤t_id); + rustc = try!(add_plugin_deps(rustc, &*build_state, plugin_deps)); + drop(build_state); // FIXME(rust-lang/rust#18913): we probably shouldn't have to do // this manually @@ -428,7 +401,76 @@ fn rustc(package: &Package, target: &Target, Ok(()) }), kind)) - }).collect() + }).collect(); + + // Add all relevant -L and -l flags from dependencies (now calculated and + // present in `state`) to the command provided + fn add_native_deps(mut rustc: CommandPrototype, + build_state: &BuildMap, + native_lib_deps: Vec, + kind: Kind, + pass_l_flag: bool, + current_id: &PackageId) -> CommandPrototype { + for id in native_lib_deps.into_iter() { + let output = &build_state[(id.clone(), kind)]; + for path in output.library_paths.iter() { + rustc = rustc.arg("-L").arg(path); + } + if pass_l_flag && id == *current_id { + for name in output.library_links.iter() { + rustc = rustc.arg("-l").arg(name.as_slice()); + } + } + } + return rustc; + } +} + +fn crawl_build_deps<'a>(cx: &'a Context, pkg: &'a Package, + target: &Target, kind: Kind) -> Vec { + let mut deps = HashSet::new(); + visit(cx, pkg, target, kind, &mut HashSet::new(), &mut deps); + let mut ret: Vec<_> = deps.into_iter().collect(); + ret.sort(); + return ret; + + fn visit<'a>(cx: &'a Context, pkg: &'a Package, target: &Target, + kind: Kind, + visiting: &mut HashSet<&'a PackageId>, + libs: &mut HashSet) { + for &(pkg, target) in cx.dep_targets(pkg, target).iter() { + let req = cx.get_requirement(pkg, target); + if !req.includes(kind) { continue } + if !visiting.insert(pkg.get_package_id()) { continue } + + if pkg.has_custom_build() { + libs.insert(pkg.get_package_id().clone()); + } + visit(cx, pkg, target, kind, visiting, libs); + visiting.remove(&pkg.get_package_id()); + } + } +} + +// For all plugin dependencies, add their -L paths (now calculated and +// present in `state`) to the dynamic library load path for the command to +// execute. +fn add_plugin_deps(rustc: CommandPrototype, + build_state: &BuildMap, + plugin_deps: Vec) + -> CargoResult { + let var = DynamicLibrary::envvar(); + let search_path = rustc.get_env(var) + .unwrap_or(CString::from_slice(b"")); + let mut search_path = os::split_paths(search_path); + for id in plugin_deps.into_iter() { + let output = &build_state[(id, Kind::Host)]; + for path in output.library_paths.iter() { + search_path.push(path.clone()); + } + } + let search_path = try!(join_paths(&search_path[], var)); + Ok(rustc.env(var, Some(search_path))) } fn prepare_rustc(package: &Package, target: &Target, crate_types: Vec<&str>, diff --git a/tests/test_cargo_compile_custom_build.rs b/tests/test_cargo_compile_custom_build.rs index 6c6a7dd5d..361f41171 100644 --- a/tests/test_cargo_compile_custom_build.rs +++ b/tests/test_cargo_compile_custom_build.rs @@ -1,4 +1,5 @@ -use std::io::File; +use std::io::{File, fs}; +use std::os; use support::{project, execs, cargo_dir}; use support::{COMPILING, RUNNING, DOCTEST}; @@ -1004,3 +1005,74 @@ test!(test_dev_dep_build_script { assert_that(p.cargo_process("test"), execs().with_status(0)); }); + +test!(build_script_with_dynamic_native_dependency { + let build = project("builder") + .file("Cargo.toml", r#" + [package] + name = "builder" + version = "0.0.1" + authors = [] + + [lib] + name = "builder" + crate-type = ["dylib"] + "#) + .file("src/lib.rs", r#" + #[no_mangle] + pub extern fn foo() {} + "#); + assert_that(build.cargo_process("build"), + execs().with_status(0).with_stderr("")); + let src = build.root().join("target"); + let lib = fs::readdir(&src).unwrap().into_iter().find(|lib| { + let lib = lib.filename_str().unwrap(); + lib.starts_with(os::consts::DLL_PREFIX) && + lib.ends_with(os::consts::DLL_SUFFIX) + }).unwrap(); + let libname = lib.filename_str().unwrap(); + let libname = libname.slice(os::consts::DLL_PREFIX.len(), + libname.len() - os::consts::DLL_SUFFIX.len()); + + let foo = project("foo") + .file("Cargo.toml", r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + build = "build.rs" + + [build-dependencies.bar] + path = "bar" + "#) + .file("build.rs", r#" + extern crate bar; + fn main() { bar::bar() } + "#) + .file("src/lib.rs", "") + .file("bar/Cargo.toml", r#" + [package] + name = "bar" + version = "0.0.1" + authors = [] + build = "build.rs" + "#) + .file("bar/build.rs", r#" + use std::os; + + fn main() { + let src = Path::new(os::getenv("SRC").unwrap()); + println!("cargo:rustc-flags=-L {}", src.dir_path().display()); + } + "#) + .file("bar/src/lib.rs", format!(r#" + pub fn bar() {{ + #[link(name = "{}")] + extern {{ fn foo(); }} + unsafe {{ foo() }} + }} + "#, libname)); + + assert_that(foo.cargo_process("build").env("SRC", Some(lib.as_vec())), + execs().with_status(0)); +}); diff --git a/tests/test_cargo_compile_plugins.rs b/tests/test_cargo_compile_plugins.rs index 7379ea16e..c246e854f 100644 --- a/tests/test_cargo_compile_plugins.rs +++ b/tests/test_cargo_compile_plugins.rs @@ -94,18 +94,6 @@ test!(plugin_with_dynamic_native_dependency { name = "builder" crate-type = ["dylib"] "#) - .file("src/main.rs", r#" - #![allow(unstable)] - use std::io::fs; - use std::os; - - fn main() { - let src = Path::new(os::getenv("SRC").unwrap()); - let dst = Path::new(os::getenv("OUT_DIR").unwrap()); - let dst = dst.join(src.filename().unwrap()); - fs::rename(&src, &dst).unwrap(); - } - "#) .file("src/lib.rs", r#" #[no_mangle] pub extern fn foo() {} @@ -138,17 +126,26 @@ test!(plugin_with_dynamic_native_dependency { fn main() {} "#) - .file("bar/Cargo.toml", format!(r#" + .file("bar/Cargo.toml", r#" [package] name = "bar" version = "0.0.1" authors = [] - build = '{}' + build = 'build.rs' [lib] name = "bar" plugin = true - "#, build.bin("builder").display())) + "#) + .file("bar/build.rs", r#" + use std::io::fs; + use std::os; + + fn main() { + let src = Path::new(os::getenv("SRC").unwrap()); + println!("cargo:rustc-flags=-L {}", src.dir_path().display()); + } + "#) .file("bar/src/lib.rs", format!(r#" #![feature(plugin_registrar)] -- 2.30.2